tools: auto fix custom eslint rule for prefer-assert-methods.js - #16652
tools: auto fix custom eslint rule for prefer-assert-methods.js#16652shobhitchittora wants to merge 2 commits into
Conversation
commented
Nov 2, 2017
|
Hi @apapirovski. Can you also review this one? Thanks in advance. |
commented
Nov 30, 2017
commented
Dec 9, 2017
|
ping @shobhitchittora — would you like to follow up on this? There's a bit of feedback here from @BridgeAR. |
commented
Dec 10, 2017
|
@apapirovski @BridgeAR extended for |
There was a problem hiding this comment.
I would not prefer assert.ok over assert. Both should be fine out of my perspective.
There was a problem hiding this comment.
I'm a bit confused here. What did you mean when you said extend for assert.ok()?
There was a problem hiding this comment.
In general assert() should be treated identical to assert.ok(). And I meant the tests should be extended to test for both. Before I commented there were only tests for assert().
There was a problem hiding this comment.
Thanks for clarifying this. I'll revert the added invalid test for assert(val).
246c566 to
74b89ca
Compare
There was a problem hiding this comment.
I could be wrong but I think the idea was that assert(foo != bar) should yield the same as assert.ok(foo != bar). Since the former is an alias for the latter. That is, they should both report an error.
(I realize that might be modifying the current rule and is somewhat outside of the scope of the original work.)
There was a problem hiding this comment.
That would indeed be nice but I guess it is best to keep that for a separate PR and I am actually about to improve the assert message for cases like that in #17581
1. Extends tests 2. Refactors code 3. Adds fixer Refs: nodejs#16636
74b89ca to
16e64b5
Compare
commented
Dec 15, 2017
|
@BridgeAR @apapirovski Updated the PR as per the new implementation by @cjihrig. |
commented
Jan 19, 2018
|
Mini-CI (enough for this test): https://ci.nodejs.org/job/node-test-commit-light/149/ |
commented
Feb 1, 2018
|
Landed in 2d6912a |
This adds eslint fixer for auto-fixing the usage of assert operators. Also adding fileoverview for the perfer-assert-methods.js file.
For example the fixer change this
assert(obj.value !== 9);toassert.notStrictEqual(obj.value, 9);Refs: #16636
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
Tools